fix: Tuple serialization#50
Merged
Merged
Conversation
…eReflectionConverter
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes tuple serialization issues by introducing a specialized TupleReflectionConverter for ValueTuple and Tuple types that implement the ITuple interface. The main problem addressed is that tuples have indexer properties from the ITuple interface that cannot be serialized directly, causing serialization failures.
Key Changes:
- Added
TupleReflectionConverterto filter out indexer properties for tuple types - Updated base converter to filter indexer properties globally
- Enhanced logging to include converter type and padding for better debugging
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ReflectorNet/src/Converter/Reflection/TupleReflectionConverter.cs | New specialized converter that handles ITuple types with higher priority than generic converter |
| ReflectorNet/src/Reflector/Reflector.Registry.cs | Registers the new TupleReflectionConverter in the converter registry |
| ReflectorNet/src/Converter/Reflection/Base/BaseReflectionConverter.cs | Adds global indexer property filtering to prevent serialization issues |
| ReflectorNet/src/Converter/Reflection/Base/BaseReflectionConverter.Serialize.cs | Improves error logging with padding and converter type information |
| ReflectorNet/src/Converter/Reflection/ArrayReflectionConverter.cs | Removes commented-out logging code |
| ReflectorNet.Tests/src/SchemaTests/TupleSchemaTests.cs | Comprehensive schema tests for tuples with 1-8 elements, nested tuples, and arrays |
| ReflectorNet.Tests/src/ReflectorTests/ValueTupleSerializationTests.cs | Serialization and round-trip tests for ValueTuple and Tuple types |
| ReflectorNet.Tests/src/ReflectorTests/ValueTuplePropertiesTests.cs | Tests for property reflection, converter selection, and indexer filtering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…tion tests and TupleReflectionConverter
…line serialization logic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a specialized converter for
ValueTupleandTupletypes and improves the handling of indexer properties during serialization. It also adds comprehensive tests forValueTuplereflection and serialization behavior. The most important changes are grouped below.Tuple and ValueTuple Serialization Improvements
TupleReflectionConverterclass that specializes in serializing types implementingITuple(such asValueTupleandTuple). This converter filters out indexer properties that cannot be serialized directly, preventing serialization issues. (ReflectorNet/src/Converter/Reflection/TupleReflectionConverter.cs)TupleReflectionConverterin theReflector.Registryto ensure it is used for relevant types. (ReflectorNet/src/Reflector/Reflector.Registry.cs)Indexer Property Filtering
ReflectorNet/src/Converter/Reflection/Base/BaseReflectionConverter.cs)Logging and Diagnostics
padding) and converter type, making debugging easier when serialization fails for fields or properties. (ReflectorNet/src/Converter/Reflection/Base/BaseReflectionConverter.Serialize.cs) [1] [2]Testing Enhancements
ValueTuplereflection and serialization, including property and field listing, interface mapping, indexer filtering, and converter selection logic. (ReflectorNet.Tests/src/ReflectorTests/ValueTuplePropertiesTests.cs)Minor Cleanup
ReflectorNet/src/Converter/Reflection/ArrayReflectionConverter.cs)